-
Notifications
You must be signed in to change notification settings - Fork 55
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
adds action that checks if an audit is required for a given PR #776
Conversation
WalkthroughA new GitHub Actions workflow, Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- .github/workflows/checkAuditRequired.yml (1 hunks)
Additional context used
actionlint
.github/workflows/checkAuditRequired.yml
36-36: shellcheck reported issue in this script: SC2086:info:8:28: Double quote to prevent globbing and word splitting
(shellcheck)
36-36: shellcheck reported issue in this script: SC2086:info:26:34: Double quote to prevent globbing and word splitting
(shellcheck)
36-36: shellcheck reported issue in this script: SC2086:info:31:33: Double quote to prevent globbing and word splitting
(shellcheck)
Additional comments not posted (5)
.github/workflows/checkAuditRequired.yml (5)
20-23
: LGTM! Repository checkout is correctly configured.The
fetch-depth: 0
ensures all history is fetched, which is necessary for the diff operation.
25-32
: LGTM! Label removal is correctly configured.The step correctly removes the specified labels from the PR.
75-81
: LGTM! Label assignment is correctly configured.The step correctly assigns the appropriate label based on the audit requirement.
83-115
: LGTM! Label verification is correctly implemented.The script ensures that exactly one of the two labels is assigned, maintaining the integrity of the labeling process.
117-117
: LGTM! Completion message is clear and informative.The message provides clear feedback on the successful completion of the GitHub Action.
run: | | ||
|
||
##### get all files modified by this PR | ||
FILES=$(git diff --name-only origin/main HEAD) | ||
|
||
##### make sure that there are modified files | ||
if [[ -z $FILES ]]; then | ||
echo -e "\033[31mNo files found. This should not happen. Please check the code of the Github action. Aborting now.\033[0m" | ||
echo "CONTINUE=false" >> $GITHUB_ENV | ||
fi | ||
|
||
##### Initialize empty variables | ||
PROTECTED_CONTRACTS="" | ||
|
||
##### go through all modified file names/paths and identify contracts with path 'src/*' | ||
while IFS= read -r FILE; do | ||
if echo "$FILE" | grep -E '^src/.*\.sol$'; then | ||
##### contract found | ||
PROTECTED_CONTRACTS="${PROTECTED_CONTRACTS}${FILE}"$'\n' | ||
fi | ||
done <<< "$FILES" | ||
|
||
##### if none found, exit here as there is nothing to do | ||
if [[ -z "$PROTECTED_CONTRACTS" ]]; then | ||
echo -e "\033[32mNo protected contracts found in Git Diff.\033[0m" | ||
echo -e "\033[32mAssigning label 'AuditNotRequired' to this PR.\033[0m" | ||
echo "AUDIT_REQUIRED=false" >> $GITHUB_ENV | ||
exit 0 | ||
else | ||
echo -e "\033[31mProtected contracts found in Git Diff.\033[0m" | ||
echo -e "\033[31mAssigning label 'AuditRequired' to this PR.\033[0m" | ||
echo "AUDIT_REQUIRED=true" >> $GITHUB_ENV | ||
fi | ||
|
||
echo "PROTECTED_CONTRACTS: $PROTECTED_CONTRACTS" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Quote variables to prevent globbing and word splitting.
The shell script contains unquoted variables, which can lead to issues with globbing and word splitting. Address the shellcheck warnings by quoting variables.
Apply this diff to quote the variables:
FILES=$(git diff --name-only origin/main HEAD)
if [[ -z "$FILES" ]]; then
echo -e "\033[31mNo files found. This should not happen. Please check the code of the Github action. Aborting now.\033[0m"
echo "CONTINUE=false" >> $GITHUB_ENV
fi
while IFS= read -r FILE; do
if echo "$FILE" | grep -E '^src/.*\.sol$'; then
PROTECTED_CONTRACTS="${PROTECTED_CONTRACTS}${FILE}"$'\n'
fi
done <<< "$FILES"
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
run: | | |
##### get all files modified by this PR | |
FILES=$(git diff --name-only origin/main HEAD) | |
##### make sure that there are modified files | |
if [[ -z $FILES ]]; then | |
echo -e "\033[31mNo files found. This should not happen. Please check the code of the Github action. Aborting now.\033[0m" | |
echo "CONTINUE=false" >> $GITHUB_ENV | |
fi | |
##### Initialize empty variables | |
PROTECTED_CONTRACTS="" | |
##### go through all modified file names/paths and identify contracts with path 'src/*' | |
while IFS= read -r FILE; do | |
if echo "$FILE" | grep -E '^src/.*\.sol$'; then | |
##### contract found | |
PROTECTED_CONTRACTS="${PROTECTED_CONTRACTS}${FILE}"$'\n' | |
fi | |
done <<< "$FILES" | |
##### if none found, exit here as there is nothing to do | |
if [[ -z "$PROTECTED_CONTRACTS" ]]; then | |
echo -e "\033[32mNo protected contracts found in Git Diff.\033[0m" | |
echo -e "\033[32mAssigning label 'AuditNotRequired' to this PR.\033[0m" | |
echo "AUDIT_REQUIRED=false" >> $GITHUB_ENV | |
exit 0 | |
else | |
echo -e "\033[31mProtected contracts found in Git Diff.\033[0m" | |
echo -e "\033[31mAssigning label 'AuditRequired' to this PR.\033[0m" | |
echo "AUDIT_REQUIRED=true" >> $GITHUB_ENV | |
fi | |
echo "PROTECTED_CONTRACTS: $PROTECTED_CONTRACTS" | |
run: | | |
##### get all files modified by this PR | |
FILES=$(git diff --name-only origin/main HEAD) | |
##### make sure that there are modified files | |
if [[ -z "$FILES" ]]; then | |
echo -e "\033[31mNo files found. This should not happen. Please check the code of the Github action. Aborting now.\033[0m" | |
echo "CONTINUE=false" >> $GITHUB_ENV | |
fi | |
##### Initialize empty variables | |
PROTECTED_CONTRACTS="" | |
##### go through all modified file names/paths and identify contracts with path 'src/*' | |
while IFS= read -r FILE; do | |
if echo "$FILE" | grep -E '^src/.*\.sol$'; then | |
##### contract found | |
PROTECTED_CONTRACTS="${PROTECTED_CONTRACTS}${FILE}"$'\n' | |
fi | |
done <<< "$FILES" | |
##### if none found, exit here as there is nothing to do | |
if [[ -z "$PROTECTED_CONTRACTS" ]]; then | |
echo -e "\033[32mNo protected contracts found in Git Diff.\033[0m" | |
echo -e "\033[32mAssigning label 'AuditNotRequired' to this PR.\033[0m" | |
echo "AUDIT_REQUIRED=false" >> $GITHUB_ENV | |
exit 0 | |
else | |
echo -e "\033[31mProtected contracts found in Git Diff.\033[0m" | |
echo -e "\033[31mAssigning label 'AuditRequired' to this PR.\033[0m" | |
echo "AUDIT_REQUIRED=true" >> $GITHUB_ENV | |
fi | |
echo "PROTECTED_CONTRACTS: $PROTECTED_CONTRACTS" |
Tools
actionlint
36-36: shellcheck reported issue in this script: SC2086:info:8:28: Double quote to prevent globbing and word splitting
(shellcheck)
36-36: shellcheck reported issue in this script: SC2086:info:26:34: Double quote to prevent globbing and word splitting
(shellcheck)
36-36: shellcheck reported issue in this script: SC2086:info:31:33: Double quote to prevent globbing and word splitting
(shellcheck)
Which Jira task belongs to this PR?
Why did I implement it this way?
Checklist before requesting a review
Checklist for reviewer (DO NOT DEPLOY and contracts BEFORE CHECKING THIS!!!)
Summary by CodeRabbit